Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RichText: pass value to store #43204

Merged
merged 21 commits into from
Dec 7, 2023
Merged

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Aug 13, 2022

What?

Currently the RichText value is passed to the store as HTML, but it can be useful to have access to the rich text value.
Solution: pass an instance of RichTextValue, which inherits from String, but has additional properties.

Currently the RichText value is passed to the store as HTML. This PR creates a new rich-text type and rich-text matcher so that rich text data can be stored directly in block attributes and the block editor store.

Why?

Storing rich text data in the block attributes and the block editor store has many advantages.

  1. We no longer need to constantly parse and serialise the HTML. This currently happens both on load and on typing. When the blocks packages parses the blocks, it actually has access to the DOM tree. What we currently do is serialise that content back to HTML (with innerHTML) and then the rich-text has to parse it again. Similarly on typing, the internal value needs to be constantly serialised before it can send the change to the store.
  2. Related to that, when something external wants to operate on rich text, it only has access to the serialised HTML, so it must parse it, do something with it, then serialise it again before setting attributes. This is not just an issue outside of core GB (like Tumblr), but also inside core. We have this problem in our splitting and merging logic (all outside of rich text, in the writing flow component), and also in footnotes. Similarly, it's tough to do Multiple blocks: Keep all text formatting icons #27221 without it.
  3. Additionally, it’s been blocking further performance improvements with rich text buttons for example. Currently this UI needs to be portalled from inside rich text, because outside of it there’s no access to the value.
  4. Having unique objects has a value too. With strings, if you have the same content, the strings are equal, so you can’t easily find and update a rich text value inside a blocks tree. With unique values, you can just loop through all attributes until you find the unique match.
  5. Having an object means that you can store additional meta information. For example, we could store the attribute key that the data saves to (currently called the identifier). This means that we can remove the identifier prop from rich text and block authors don’t need to think about it. It’s can be a source of bugs because the identifier is easily forgotten (even in core blocks), and you could give a wrong identifier/attribute keys. What’s worse is that it's again only accessible to the rich text instance. In order to make further improvements to writing flow, we’ll need to store these identifiers in the DOM so that they’re known outside of rich text (Writing flow: absorb partial multi selection dispatching #47525).
  6. Similarly, we'd no longer have insert special characters like START_OF_SELECTED_AREA to keep track of the selection when splitting and merging, because we can now simply set a property to the object.
  7. Annotation/comments that are stored within the rich text value (but removed on serialisation) will now also be stored in the block attributes, which means they'll be preserved when splitting and merging rich text instances!
  8. Currently, the start/end selection index in the store actually refers to the rich text selection index, while the content is stored as HTML, so these indices are useless without converting to a rich text value.
  9. Further, the idea is to add rich text manipulation tools directly to the class as methods (similar to string methods), making it even easier to work with.

How?

With a new rich-text type and source added to the block attribute schema.

Testing Instructions

Screenshots or screencast

@github-actions
Copy link

github-actions bot commented Aug 13, 2022

Size Change: +972 B (0%)

Total Size: 1.72 MB

Filename Size Change
build/block-editor/index.min.js 248 kB +67 B (0%)
build/block-library/index.min.js 213 kB +196 B (0%)
build/blocks/index.min.js 51.3 kB +223 B (0%)
build/core-data/index.min.js 72.6 kB +17 B (0%)
build/rich-text/index.min.js 10.5 kB +469 B (+5%) 🔍
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 964 B
build/annotations/index.min.js 2.71 kB
build/api-fetch/index.min.js 2.29 kB
build/autop/index.min.js 2.11 kB
build/blob/index.min.js 590 B
build/block-directory/index.min.js 7.25 kB
build/block-directory/style-rtl.css 1.04 kB
build/block-directory/style.css 1.04 kB
build/block-editor/content-rtl.css 4.29 kB
build/block-editor/content.css 4.28 kB
build/block-editor/default-editor-styles-rtl.css 403 B
build/block-editor/default-editor-styles.css 403 B
build/block-editor/style-rtl.css 15.7 kB
build/block-editor/style.css 15.7 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 122 B
build/block-library/blocks/audio/style.css 122 B
build/block-library/blocks/audio/theme-rtl.css 138 B
build/block-library/blocks/audio/theme.css 138 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 104 B
build/block-library/blocks/avatar/style.css 104 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/button/editor-rtl.css 587 B
build/block-library/blocks/button/editor.css 587 B
build/block-library/blocks/button/style-rtl.css 633 B
build/block-library/blocks/button/style.css 632 B
build/block-library/blocks/buttons/editor-rtl.css 337 B
build/block-library/blocks/buttons/editor.css 337 B
build/block-library/blocks/buttons/style-rtl.css 332 B
build/block-library/blocks/buttons/style.css 332 B
build/block-library/blocks/calendar/style-rtl.css 239 B
build/block-library/blocks/calendar/style.css 239 B
build/block-library/blocks/categories/editor-rtl.css 113 B
build/block-library/blocks/categories/editor.css 112 B
build/block-library/blocks/categories/style-rtl.css 124 B
build/block-library/blocks/categories/style.css 124 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 121 B
build/block-library/blocks/code/style.css 121 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 421 B
build/block-library/blocks/columns/style.css 421 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 199 B
build/block-library/blocks/comment-template/style.css 198 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 840 B
build/block-library/blocks/comments/editor.css 839 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 636 B
build/block-library/blocks/cover/editor-rtl.css 647 B
build/block-library/blocks/cover/editor.css 650 B
build/block-library/blocks/cover/style-rtl.css 1.7 kB
build/block-library/blocks/cover/style.css 1.69 kB
build/block-library/blocks/details/editor-rtl.css 65 B
build/block-library/blocks/details/editor.css 65 B
build/block-library/blocks/details/style-rtl.css 98 B
build/block-library/blocks/details/style.css 98 B
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 138 B
build/block-library/blocks/embed/theme.css 138 B
build/block-library/blocks/file/editor-rtl.css 316 B
build/block-library/blocks/file/editor.css 316 B
build/block-library/blocks/file/style-rtl.css 280 B
build/block-library/blocks/file/style.css 281 B
build/block-library/blocks/file/view.min.js 322 B
build/block-library/blocks/footnotes/style-rtl.css 201 B
build/block-library/blocks/footnotes/style.css 199 B
build/block-library/blocks/form-input/editor-rtl.css 229 B
build/block-library/blocks/form-input/editor.css 228 B
build/block-library/blocks/form-input/style-rtl.css 343 B
build/block-library/blocks/form-input/style.css 343 B
build/block-library/blocks/form-submission-notification/editor-rtl.css 343 B
build/block-library/blocks/form-submission-notification/editor.css 342 B
build/block-library/blocks/form-submit-button/style-rtl.css 69 B
build/block-library/blocks/form-submit-button/style.css 69 B
build/block-library/blocks/form/view.min.js 452 B
build/block-library/blocks/freeform/editor-rtl.css 2.61 kB
build/block-library/blocks/freeform/editor.css 2.61 kB
build/block-library/blocks/gallery/editor-rtl.css 957 B
build/block-library/blocks/gallery/editor.css 962 B
build/block-library/blocks/gallery/style-rtl.css 1.75 kB
build/block-library/blocks/gallery/style.css 1.75 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 654 B
build/block-library/blocks/group/editor.css 654 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 189 B
build/block-library/blocks/heading/style.css 189 B
build/block-library/blocks/html/editor-rtl.css 340 B
build/block-library/blocks/html/editor.css 341 B
build/block-library/blocks/image/editor-rtl.css 834 B
build/block-library/blocks/image/editor.css 833 B
build/block-library/blocks/image/style-rtl.css 1.61 kB
build/block-library/blocks/image/style.css 1.6 kB
build/block-library/blocks/image/theme-rtl.css 137 B
build/block-library/blocks/image/theme.css 137 B
build/block-library/blocks/image/view.min.js 2.02 kB
build/block-library/blocks/latest-comments/style-rtl.css 357 B
build/block-library/blocks/latest-comments/style.css 357 B
build/block-library/blocks/latest-posts/editor-rtl.css 213 B
build/block-library/blocks/latest-posts/editor.css 212 B
build/block-library/blocks/latest-posts/style-rtl.css 478 B
build/block-library/blocks/latest-posts/style.css 478 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 505 B
build/block-library/blocks/media-text/style.css 503 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 671 B
build/block-library/blocks/navigation-link/editor.css 672 B
build/block-library/blocks/navigation-link/style-rtl.css 103 B
build/block-library/blocks/navigation-link/style.css 103 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation/editor-rtl.css 2.26 kB
build/block-library/blocks/navigation/editor.css 2.26 kB
build/block-library/blocks/navigation/style-rtl.css 2.27 kB
build/block-library/blocks/navigation/style.css 2.26 kB
build/block-library/blocks/navigation/view.min.js 1.04 kB
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 401 B
build/block-library/blocks/page-list/editor.css 401 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 235 B
build/block-library/blocks/paragraph/editor.css 235 B
build/block-library/blocks/paragraph/style-rtl.css 335 B
build/block-library/blocks/paragraph/style.css 335 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 508 B
build/block-library/blocks/post-comments-form/style.css 508 B
build/block-library/blocks/post-date/style-rtl.css 61 B
build/block-library/blocks/post-date/style.css 61 B
build/block-library/blocks/post-excerpt/editor-rtl.css 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 141 B
build/block-library/blocks/post-excerpt/style.css 141 B
build/block-library/blocks/post-featured-image/editor-rtl.css 666 B
build/block-library/blocks/post-featured-image/editor.css 662 B
build/block-library/blocks/post-featured-image/style-rtl.css 345 B
build/block-library/blocks/post-featured-image/style.css 345 B
build/block-library/blocks/post-navigation-link/style-rtl.css 215 B
build/block-library/blocks/post-navigation-link/style.css 214 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 409 B
build/block-library/blocks/post-template/style.css 408 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-time-to-read/style-rtl.css 69 B
build/block-library/blocks/post-time-to-read/style.css 69 B
build/block-library/blocks/post-title/style-rtl.css 100 B
build/block-library/blocks/post-title/style.css 100 B
build/block-library/blocks/preformatted/style-rtl.css 125 B
build/block-library/blocks/preformatted/style.css 125 B
build/block-library/blocks/pullquote/editor-rtl.css 135 B
build/block-library/blocks/pullquote/editor.css 135 B
build/block-library/blocks/pullquote/style-rtl.css 335 B
build/block-library/blocks/pullquote/style.css 335 B
build/block-library/blocks/pullquote/theme-rtl.css 168 B
build/block-library/blocks/pullquote/theme.css 168 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 288 B
build/block-library/blocks/query-pagination/style.css 284 B
build/block-library/blocks/query-title/style-rtl.css 63 B
build/block-library/blocks/query-title/style.css 63 B
build/block-library/blocks/query/editor-rtl.css 486 B
build/block-library/blocks/query/editor.css 486 B
build/block-library/blocks/query/style-rtl.css 312 B
build/block-library/blocks/query/style.css 308 B
build/block-library/blocks/query/view.min.js 647 B
build/block-library/blocks/quote/style-rtl.css 237 B
build/block-library/blocks/quote/style.css 237 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 140 B
build/block-library/blocks/read-more/style.css 140 B
build/block-library/blocks/rss/editor-rtl.css 149 B
build/block-library/blocks/rss/editor.css 149 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 184 B
build/block-library/blocks/search/editor.css 184 B
build/block-library/blocks/search/style-rtl.css 613 B
build/block-library/blocks/search/style.css 613 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/search/view.min.js 475 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 234 B
build/block-library/blocks/separator/style.css 234 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 329 B
build/block-library/blocks/shortcode/editor.css 329 B
build/block-library/blocks/site-logo/editor-rtl.css 760 B
build/block-library/blocks/site-logo/editor.css 760 B
build/block-library/blocks/site-logo/style-rtl.css 204 B
build/block-library/blocks/site-logo/style.css 204 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 116 B
build/block-library/blocks/site-title/editor.css 116 B
build/block-library/blocks/site-title/style-rtl.css 57 B
build/block-library/blocks/site-title/style.css 57 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 682 B
build/block-library/blocks/social-links/editor.css 681 B
build/block-library/blocks/social-links/style-rtl.css 1.49 kB
build/block-library/blocks/social-links/style.css 1.49 kB
build/block-library/blocks/spacer/editor-rtl.css 359 B
build/block-library/blocks/spacer/editor.css 359 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 432 B
build/block-library/blocks/table/editor.css 432 B
build/block-library/blocks/table/style-rtl.css 646 B
build/block-library/blocks/table/style.css 645 B
build/block-library/blocks/table/theme-rtl.css 157 B
build/block-library/blocks/table/theme.css 157 B
build/block-library/blocks/tag-cloud/style-rtl.css 251 B
build/block-library/blocks/tag-cloud/style.css 253 B
build/block-library/blocks/template-part/editor-rtl.css 403 B
build/block-library/blocks/template-part/editor.css 403 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/term-description/style-rtl.css 111 B
build/block-library/blocks/term-description/style.css 111 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 99 B
build/block-library/blocks/verse/style.css 99 B
build/block-library/blocks/video/editor-rtl.css 552 B
build/block-library/blocks/video/editor.css 555 B
build/block-library/blocks/video/style-rtl.css 191 B
build/block-library/blocks/video/style.css 191 B
build/block-library/blocks/video/theme-rtl.css 139 B
build/block-library/blocks/video/theme.css 139 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.11 kB
build/block-library/common.css 1.11 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 12.5 kB
build/block-library/editor.css 12.4 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 472 B
build/block-library/reset.css 472 B
build/block-library/style-rtl.css 14.7 kB
build/block-library/style.css 14.7 kB
build/block-library/theme-rtl.css 700 B
build/block-library/theme.css 705 B
build/block-serialization-default-parser/index.min.js 1.13 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/commands/index.min.js 15.5 kB
build/commands/style-rtl.css 947 B
build/commands/style.css 942 B
build/components/index.min.js 256 kB
build/components/style-rtl.css 12.1 kB
build/components/style.css 12.1 kB
build/compose/index.min.js 12.8 kB
build/core-commands/index.min.js 2.73 kB
build/customize-widgets/index.min.js 12.1 kB
build/customize-widgets/style-rtl.css 1.43 kB
build/customize-widgets/style.css 1.43 kB
build/data-controls/index.min.js 651 B
build/data/index.min.js 8.87 kB
build/date/index.min.js 17.9 kB
build/deprecated/index.min.js 462 B
build/dom-ready/index.min.js 336 B
build/dom/index.min.js 4.68 kB
build/edit-post/classic-rtl.css 571 B
build/edit-post/classic.css 571 B
build/edit-post/index.min.js 34.9 kB
build/edit-post/style-rtl.css 7.56 kB
build/edit-post/style.css 7.55 kB
build/edit-site/index.min.js 210 kB
build/edit-site/style-rtl.css 14.6 kB
build/edit-site/style.css 14.6 kB
build/edit-widgets/index.min.js 17.2 kB
build/edit-widgets/style-rtl.css 4.65 kB
build/edit-widgets/style.css 4.64 kB
build/editor/index.min.js 48.6 kB
build/editor/style-rtl.css 3.74 kB
build/editor/style.css 3.73 kB
build/element/index.min.js 4.87 kB
build/escape-html/index.min.js 548 B
build/format-library/index.min.js 7.76 kB
build/format-library/style-rtl.css 577 B
build/format-library/style.css 577 B
build/hooks/index.min.js 1.57 kB
build/html-entities/index.min.js 454 B
build/i18n/index.min.js 3.61 kB
build/interactivity/file.min.js 442 B
build/interactivity/image.min.js 2.15 kB
build/interactivity/index.min.js 12.5 kB
build/interactivity/navigation.min.js 1.16 kB
build/interactivity/query.min.js 791 B
build/interactivity/search.min.js 610 B
build/is-shallow-equal/index.min.js 535 B
build/keyboard-shortcuts/index.min.js 1.76 kB
build/keycodes/index.min.js 1.9 kB
build/list-reusable-blocks/index.min.js 2.11 kB
build/list-reusable-blocks/style-rtl.css 865 B
build/list-reusable-blocks/style.css 865 B
build/media-utils/index.min.js 2.92 kB
build/modules/importmap-polyfill.min.js 12.2 kB
build/notices/index.min.js 964 B
build/nux/index.min.js 2.01 kB
build/nux/style-rtl.css 775 B
build/nux/style.css 771 B
build/patterns/index.min.js 5.28 kB
build/patterns/style-rtl.css 564 B
build/patterns/style.css 564 B
build/plugins/index.min.js 1.81 kB
build/preferences-persistence/index.min.js 1.85 kB
build/preferences/index.min.js 1.26 kB
build/primitives/index.min.js 994 B
build/priority-queue/index.min.js 1.52 kB
build/private-apis/index.min.js 994 B
build/react-i18n/index.min.js 631 B
build/react-refresh-entry/index.min.js 9.46 kB
build/react-refresh-runtime/index.min.js 6.78 kB
build/redux-routine/index.min.js 2.71 kB
build/reusable-blocks/index.min.js 2.74 kB
build/reusable-blocks/style-rtl.css 265 B
build/reusable-blocks/style.css 265 B
build/router/index.min.js 1.79 kB
build/server-side-render/index.min.js 1.96 kB
build/shortcode/index.min.js 1.4 kB
build/style-engine/index.min.js 1.98 kB
build/token-list/index.min.js 587 B
build/url/index.min.js 3.83 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 41.8 kB
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 967 B
build/warning/index.min.js 259 B
build/widgets/index.min.js 7.18 kB
build/widgets/style-rtl.css 1.18 kB
build/widgets/style.css 1.18 kB
build/wordcount/index.min.js 1.03 kB

compressed-size-action

@ellatrix ellatrix force-pushed the try/pass-rich-text-value-to-store branch from db12b70 to 54e3708 Compare August 13, 2022 01:24
@ellatrix ellatrix force-pushed the try/pass-rich-text-value-to-store branch from 1be952f to 62cdd52 Compare August 13, 2022 08:56
@ellatrix ellatrix marked this pull request as ready for review August 13, 2022 21:51
@skorasaurus skorasaurus added the [Package] Rich text /packages/rich-text label Aug 15, 2022
@ellatrix ellatrix force-pushed the try/pass-rich-text-value-to-store branch from 76135e8 to edf956e Compare May 3, 2023 13:30
packages/rich-text/src/value.js Outdated Show resolved Hide resolved
packages/rich-text/src/component/index.js Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented May 3, 2023

Flaky tests detected in bce8a2d.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7112599031
📝 Reported issues:

@ellatrix ellatrix force-pushed the try/pass-rich-text-value-to-store branch from 27389f8 to fc88c6e Compare May 4, 2023 07:12
@dmsnell dmsnell force-pushed the try/pass-rich-text-value-to-store branch 2 times, most recently from 540caa8 to 310d80f Compare May 25, 2023 09:11
@ellatrix ellatrix force-pushed the try/pass-rich-text-value-to-store branch from 310d80f to 1aab304 Compare August 11, 2023 18:18
@ellatrix ellatrix added the [Type] Code Quality Issues or PRs that relate to code quality label Aug 11, 2023
@ellatrix ellatrix force-pushed the try/pass-rich-text-value-to-store branch from bff3746 to 1c09d7c Compare September 11, 2023 16:21
@ellatrix ellatrix force-pushed the try/pass-rich-text-value-to-store branch from 15b4e94 to 14edd7a Compare November 9, 2023 18:25
@ellatrix ellatrix force-pushed the try/pass-rich-text-value-to-store branch from 99600d3 to bce8a2d Compare December 6, 2023 09:26
Copy link
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR doesn't need a second birthday party. This is good timing since a release just went out, and I think every major item has been addressed, so it's good time to get this out the door and in testing.

Maybe we can remain paced at further changes so we have an extended period of time to catch any unforeseen bugs before it becomes harder to revert should we need.

I'm looking forward to this and even more to some later follow-up work with things like .toReactComponent() that can further bypass printing/parsing steps.

Well done, @ellatrix - this took a lot of persistence, a lot of preparation work in other PRs, and a lot of experimentation. I hope it goes well.

:shipit:

@@ -6,5 +6,6 @@
* @return {string} The value with anchor tags removed.
*/
export default function removeAnchorTag( value ) {
return value.replace( /<\/?a[^>]*>/g, '' );
// To do: Refactor this to use rich text's removeFormat instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or at least a parsed version of the HTML! in another PR. This breaks as-is in trivial cases (e.g. <a title="love > hate">) but the breakage isn't introduced in this PR so we don't have to fix it here either


if ( /[\n\t\r\f]/.test( newNodeValue ) ) {
newNodeValue = newNodeValue.replace( /[\n\t\r\f]+/g, ' ' );
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any reason to run this test before the replace, because the test has to scan the same content that the replace is going to do. if there are none of these characters then the test will still scan the entire string. so there are no cases where the test is going to save us work that the replace itself would be doing. we might as well always run the replace.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it might be slightly faster. Either way, most of the time there shouldn't be any of these characters present.

@ellatrix
Copy link
Member Author

ellatrix commented Dec 7, 2023

Thanks @dmsnell! Let's indeed merge now that it's still early. I'll be available to deal with any unforeseen consequences and do everything we can to avoid a revert. :)

@ellatrix ellatrix merged commit 6a42225 into trunk Dec 7, 2023
52 checks passed
@ellatrix ellatrix deleted the try/pass-rich-text-value-to-store branch December 7, 2023 20:41
@github-actions github-actions bot added this to the Gutenberg 17.3 milestone Dec 7, 2023
dcalhoun added a commit that referenced this pull request Dec 13, 2023
In this context, `this.value` is not a string but a instance of
`RichTextData`. Therefore, comparing the two values results in
unexpected inequality, triggering an update of the block's
`attributes.content` toggling it from a `ReactTextData` instance to a
string. This toggle results in the undo manager tracking the change as
a new line of editor history, clearing out any pending redo actions.

The `RichTextData` type was introduced in #43204. Invoking `toString`
may not be the best long-term solution to this problem. Refactoring the
rich text implementation to appropriately leverage `RichTextData` and
(potentially) treat `value` and `record` values different and storing
them separately may be necessary.
dcalhoun added a commit that referenced this pull request Dec 14, 2023
* fix: Prevent unnecessary content changes clearing redo actions

In this context, `this.value` is not a string but a instance of
`RichTextData`. Therefore, comparing the two values results in
unexpected inequality, triggering an update of the block's
`attributes.content` toggling it from a `ReactTextData` instance to a
string. This toggle results in the undo manager tracking the change as
a new line of editor history, clearing out any pending redo actions.

The `RichTextData` type was introduced in #43204. Invoking `toString`
may not be the best long-term solution to this problem. Refactoring the
rich text implementation to appropriately leverage `RichTextData` and
(potentially) treat `value` and `record` values different and storing
them separately may be necessary.

* fix: Convert `RichTextData` to string before comparing values

The value stored in the rich text component may be a string or a
`RichTextData`. Until the value is store consistently, it may be
necessary to convert each value to a string prior to equality
comparisons.

* test: Verify change events with equal values do not update attributes

Ensure empty string values do not cause unnecessary attribute updates
when comparing string values to empty `RichTextData` values, which is
the new default value.
@kevin940726
Copy link
Member

This PR seems to break the TOC block. Adding a TOC block and a heading block to the editor throws the below error.

Uncaught TypeError: headingAttributes.content.replace is not a function
    at getLatestHeadings (hooks.js:108:33)

content is now a RichTextData class and not a string. Possibly an oversight?

@dcalhoun
Copy link
Member

This PR seems to break the TOC block. Adding a TOC block and a heading block to the editor throws the below error.

Uncaught TypeError: headingAttributes.content.replace is not a function
    at getLatestHeadings (hooks.js:108:33)

content is now a RichTextData class and not a string. Possibly an oversight?

Curious to see thoughts on this. While not a block, the native mobile rich text feature experienced similar issues that we addressed in #57028, but we are discovering more breakages that we need to address.

What is the likelihood that third-party blocks break because of this change as well? Do we consider this change of content type a breaking change that needs to be communicated?

@ellatrix
Copy link
Member Author

I commented on this in #57093. We could add all the string methods for back compat if needed. However, it seems quite rare to be touching another block's attributes. Third party blocks by themselves can breaks because the value is opt in, it only happens when you use attributes from other blocks. I'm happy to add string methods though, it's not a big deal.

artemiomorales pushed a commit that referenced this pull request Jan 4, 2024
* fix: Prevent unnecessary content changes clearing redo actions

In this context, `this.value` is not a string but a instance of
`RichTextData`. Therefore, comparing the two values results in
unexpected inequality, triggering an update of the block's
`attributes.content` toggling it from a `ReactTextData` instance to a
string. This toggle results in the undo manager tracking the change as
a new line of editor history, clearing out any pending redo actions.

The `RichTextData` type was introduced in #43204. Invoking `toString`
may not be the best long-term solution to this problem. Refactoring the
rich text implementation to appropriately leverage `RichTextData` and
(potentially) treat `value` and `record` values different and storing
them separately may be necessary.

* fix: Convert `RichTextData` to string before comparing values

The value stored in the rich text component may be a string or a
`RichTextData`. Until the value is store consistently, it may be
necessary to convert each value to a string prior to equality
comparisons.

* test: Verify change events with equal values do not update attributes

Ensure empty string values do not cause unnecessary attribute updates
when comparing string values to empty `RichTextData` values, which is
the new default value.
@t-hamano
Copy link
Contributor

t-hamano commented Feb 5, 2024

I discovered that this PR causes problems for blocks with pre tags. The problem is summarized in #58659. Is it possible to return the RichText format to the previous format in blocks with pre tags, i.e. Code, Preformatted, Verse blocks?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Rich text /packages/rich-text [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants